Skip to content

fix: balance SV refcount in pp_overload_ft_nv#1

Closed
Koan-Bot wants to merge 12 commits intomasterfrom
koan.atoomic/fix-sv-refcount-leak
Closed

fix: balance SV refcount in pp_overload_ft_nv#1
Koan-Bot wants to merge 12 commits intomasterfrom
koan.atoomic/fix-sv-refcount-leak

Conversation

@Koan-Bot
Copy link
Copy Markdown
Collaborator

@Koan-Bot Koan-Bot commented Mar 7, 2026

What

Add missing SvREFCNT_dec(status) calls in pp_overload_ft_nv to balance the SvREFCNT_inc done in _overload_ft_ops_sv().

Why

_overload_ft_ops_sv() increments the refcount of the returned SV to keep it alive past FREETMPS, but pp_overload_ft_nv (the only caller) never decremented it — leaking one SV per mocked -M/-C/-A call.

How

Added SvREFCNT_dec(status) before each of the three return paths in pp_overload_ft_nv: the two fallback-to-real-op paths (when value is -1) and the normal TARG return path.

Testing

Full test suite passes (1072 tests, 50 files).

Koan-Bot and others added 12 commits February 22, 2026 00:53
_check() stored the file argument in $_last_call_for for -X _ caching.
When the argument was a filehandle reference (not a string path), this
prevented the filehandle from being garbage collected, keeping the
underlying file descriptor open.

This caused "spooky action-at-a-distance" bugs: e.g. a socketpair read
hanging because a dup'd write-end filehandle was kept alive by the
leaked reference, even after leaving scope.

Fix: only cache string filenames in $_last_call_for, not references.

Ref: cpan-authors/Test-MockFile#179

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ef-leak

fix: prevent filehandle reference leak in stat override
Add CLAUDE.md with project guidance for Claude Code sessions.
Exclude CLAUDE.md and local/ directory from Dist::Zilla GatherDir
so they don't end up in the CPAN tarball.
_overload_ft_ops_sv() increments the refcount of the returned SV to
keep it alive past FREETMPS, but pp_overload_ft_nv never decremented
it, leaking one SV per mocked -M/-C/-A call.

Add SvREFCNT_dec(status) before each return path in pp_overload_ft_nv.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@atoomic atoomic marked this pull request as ready for review March 7, 2026 21:58
@atoomic
Copy link
Copy Markdown
Owner

atoomic commented Mar 7, 2026

We need to close that Pull Request and resubmit it to the parent repo https://github.com/cpanel/Overload-FileCheck

@atoomic
Copy link
Copy Markdown
Owner

atoomic commented Mar 7, 2026

@Koan-Bot recreate

@atoomic
Copy link
Copy Markdown
Owner

atoomic commented Apr 23, 2026

@Koan-Bot squash

@Koan-Bot
Copy link
Copy Markdown
Collaborator Author

This fix has already been applied to upstream/main (cpan-authors/Overload-FileCheck). Closing as superseded.

@Koan-Bot Koan-Bot closed this Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants